Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Add room_version param to get_pdu #4448

Merged
merged 3 commits into from
Jan 24, 2019
Merged

Conversation

erikjohnston
Copy link
Member

When we add new event format we'll need to know the event format or room
version when parsing events.

When we add new event format we'll need to know the event format or room
version when parsing events.
@codecov-io
Copy link

codecov-io commented Jan 23, 2019

Codecov Report

Merging #4448 into develop will increase coverage by 1.16%.
The diff coverage is 70.37%.

@@            Coverage Diff             @@
##           develop   #4448      +/-   ##
==========================================
+ Coverage    73.64%   74.8%   +1.16%     
==========================================
  Files          302     336      +34     
  Lines        29818   34804    +4986     
  Branches      4895    5800     +905     
==========================================
+ Hits         21960   26036    +4076     
- Misses        6426    7149     +723     
- Partials      1432    1619     +187

@erikjohnston
Copy link
Member Author

Split out from #4403 and address this review comment about making sure get_pdu knows the correct room version

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks generally sane, modulo question below

@@ -1659,6 +1664,8 @@ def _persist_auth_tree(self, origin, auth_events, state, event):
create_event = e
break

room_version = create_event.content.get("room_version", RoomVersions.V1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it possible for us to get here without a create_event (if the auth chain is faulty), in which case this will explode rather than failing sensibly?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe so, but may as well add a check anyway.

break

if room_version is None:
# We use this error has that is what
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deep.

@erikjohnston erikjohnston merged commit edc1e21 into develop Jan 24, 2019
@erikjohnston erikjohnston deleted the erikj/get_pdu_versions branch March 5, 2019 13:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants